fix(skirmish): Improve determinism for restarted games by resetting slot values by Caball009 · Pull Request #2373 · TheSuperHackers/GeneralsGameCode

Caball009 · 2026-03-01T14:55:33Z

Restarted skirmish games may not start with the same logical seed value as the first start. This depends on whether there are players with a random color / position / faction. Those random values are determined by using and updating the logical seed on the first start, after which the random values are stored. That means that for restarted games the logical seed isn't used or updated for those purposes. This PR resets those values to improve determinism for restarted games.

You can put a breakpoint after GameLogic::startNewGame and compare the values of theGameLogicSeed for the first start and following starts and see how the values for the first start deviate from restarts.

TODO:

Address feedback.
Replicate in Generals.

greptile-apps · 2026-03-19T13:53:15Z

Greptile Summary

This PR fixes a determinism issue in skirmish game restarts by ensuring the logical seed is consumed in the same way on every start, whether it's the first launch or a restart.\n\nRoot cause: When random color/position/faction options are chosen, the first startNewGame() call uses the logical seed to resolve them, storing the concrete values and advancing the seed. On a restart those concrete values were already set, so the seed was never advanced for them — causing the seed state to diverge from the first run and producing different game outcomes.\n\nFix approach:\n- A m_saveOriginalSetup flag is added to GameSlot (initialised TRUE, cleared to FALSE after the first saveOriginalSetup() call).\n- On the first startNewGame() call the original (potentially -1 / random) values are saved as before.\n- On subsequent calls (restart path), the original values — including -1 for random slots — are restored before populateRandomSideAndColor() / populateRandomStartPosition() run, so the seed is consumed identically.\n- A toString(GameMode) helper is introduced in Zero Hour's GameLogic to provide a human-readable mode name in the new DEBUG_ASSERTCRASH.\n- The xfer (save/load) path is correctly handled: reset() is called then saveOriginalSetup() is called during load, leaving m_saveOriginalSetup = FALSE so subsequent restarts take the correct branch.\n\nKey findings:\n- The getSaveOriginalSetup() getter has inverted boolean semantics (TRUE = not yet saved, FALSE = already saved), leading to a double-negative if (!slot->getSaveOriginalSetup()) at the call-site. Renaming to isOriginalSetupSaved() with matching field/initialiser inversion would improve clarity.\n- The fix is only applied to Zero Hour (GeneralsMD/) for now; the PR TODO acknowledges that vanilla Generals still needs to be updated.

Confidence Score: 5/5

Safe to merge; the determinism fix is logically sound, the xfer/save-load path is handled correctly, and the only remaining finding is a P2 naming clarity nit.

All four changed files are straightforward, no runtime-breaking issues were identified, and the core invariant (restoring -1 random-marker values before seed consumption) is correct. The one comment is a style suggestion on getter naming that does not affect behavior.

No files require special attention; consider the naming clarity suggestion on getSaveOriginalSetup() in GameInfo.h before merging if code readability is a priority.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameNetwork/GameInfo.h	Adds m_saveOriginalSetup flag and renames saveOffOriginalInfo to saveOriginalSetup; minor naming clarity concern on the getter.
Core/GameEngine/Source/GameNetwork/GameInfo.cpp	Implements m_saveOriginalSetup flag in reset() and saveOriginalSetup(); xfer load correctly calls saveOriginalSetup() to set the flag after restoring original values.
GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h	Adds toString(GameMode) free-function declaration; straightforward, no issues.
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Core fix: startNewGame() now restores original slot values (color/pos/template = -1 for random) on restart so the logical seed is consumed identically to the first start; toString(GameMode) implementation covers all enum values.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[startNewGame called] --> B{loadingSaveGame?}
    B -- Yes --> E[Skip slot setup]
    B -- No --> C{m_saveOriginalSetup == TRUE?}

    C -- "TRUE: first start" --> D["saveOriginalSetup\nstores color/pos/template as originals\nsets m_saveOriginalSetup = FALSE"]
    C -- "FALSE: restart" --> F["Assert GAME_SKIRMISH\nRestore originals\nsetColor / setStartPos / setPlayerTemplate\nmay restore -1 for random slots"]

    D --> G[populateRandomSideAndColor\npopulateRandomStartPosition]
    F --> G
    E --> G

    G --> H[Logical seed consumed\nidentically on every start]

Prompt To Fix All With AI

This is a comment left during a code review.
Path: Core/GameEngine/Include/GameNetwork/GameInfo.h
Line: 104

Comment:
**Confusing boolean semantics on `getSaveOriginalSetup()`**

The field `m_saveOriginalSetup` is initialised to `TRUE` (meaning "not yet saved") and flipped to `FALSE` after `saveOriginalSetup()` runs. This causes the caller in `GameLogic.cpp` to use a double-negative: `if (!slot->getSaveOriginalSetup())` to detect the restart path.

Consider naming the field/getter to reflect the completed state, which reads naturally at the call-site:

```suggestion
	Bool isOriginalSetupSaved() const { return m_saveOriginalSetup; }
```

And rename the member to `m_isOriginalSetupSaved`, initialised to `FALSE` in `reset()` and set to `TRUE` in `saveOriginalSetup()`. Then `GameLogic.cpp` becomes `if (slot->isOriginalSetupSaved())` — immediately clear that this is the restart path.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (4): Last reviewed commit: "Reset randomized slot values." | Re-trigger Greptile}

xezon

OK

Needs to be replicated in Generals

GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp

Core/GameEngine/Source/GameNetwork/GameInfo.cpp

GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp

xezon

Greptile is still complaining about something?

GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h

Caball009 · 2026-03-22T14:08:36Z

Greptile is still complaining about something?

Yes, that was a good point. I think it could have affected the randomization of AI players if a game was loaded from inside another game or replay.

GeneralsGameCode/Core/GameEngine/Source/GameNetwork/GameInfo.cpp

Lines 1594 to 1600 in 637dbba

    
           m_slot[slot]->setState((SlotState)state, name); 
        
           if (isAccepted) m_slot[slot]->setAccept(); 
        
           m_slot[slot]->setPlayerTemplate(origPlayerTemplate); 
        
           m_slot[slot]->setStartPos(origStartPos); 
        
           m_slot[slot]->setColor(origColor); 
        
           m_slot[slot]->saveOffOriginalInfo();

It should work correctly now because GameSlot::setState sets m_saveOffOriginalInfo back to true before GameSlot::saveOffOriginalInfo is called.

Replication in Generals still needs doing.

xezon · 2026-03-23T19:40:28Z

Core/GameEngine/Source/GameNetwork/GameInfo.cpp

 	}
 	else
 	{
+		m_saveOffOriginalInfo = TRUE;


The reason for this is unintuitive.

Perhaps it would be better to have 2 versions of saveOffOriginalInfo, one used in the xfer and the other on new game start? Or just test the boolean outside of saveOffOriginalInfo.

That may also be a good opportunity to fix the name of saveOffOriginalInfo, for example saveOriginalSetup.

Ok, changed.

Caball009 · 2026-03-27T21:50:48Z

I can replicate in Generals unless there are other changes desired.

Caball009 added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Fix Is fixing something, but is not user facing labels Mar 1, 2026

Caball009 marked this pull request as ready for review March 19, 2026 13:50

xezon approved these changes Mar 19, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp Outdated Show resolved Hide resolved

Core/GameEngine/Source/GameNetwork/GameInfo.cpp Outdated Show resolved Hide resolved

GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp Outdated Show resolved Hide resolved

xezon approved these changes Mar 21, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h Outdated Show resolved Hide resolved

xezon reviewed Mar 23, 2026

View reviewed changes

Caball009 added 3 commits March 27, 2026 15:52

Added 'toString' function.

ee50e97

Changed function name from 'saveOffOriginalInfo' to 'saveOriginalSetup'.

d5597b7

Reset randomized slot values.

0239428

Caball009 force-pushed the fix_reset_slot_values_for_logical_seed branch from e5d4698 to 0239428 Compare March 27, 2026 15:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(skirmish): Improve determinism for restarted games by resetting slot values#2373

fix(skirmish): Improve determinism for restarted games by resetting slot values#2373
Caball009 wants to merge 3 commits intoTheSuperHackers:mainfrom
Caball009:fix_reset_slot_values_for_logical_seed

Caball009 commented Mar 1, 2026 •

edited

Loading

Uh oh!

greptile-apps bot commented Mar 19, 2026 •

edited

Loading

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

xezon left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xezon left a comment

Uh oh!

Uh oh!

Caball009 commented Mar 22, 2026

Uh oh!

xezon Mar 23, 2026

Uh oh!

Caball009 Mar 27, 2026

Uh oh!

Caball009 commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Caball009 commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO:

Uh oh!

greptile-apps bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Caball009 commented Mar 22, 2026

Uh oh!

xezon Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Caball009 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Caball009 commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Caball009 commented Mar 1, 2026 •

edited

Loading

greptile-apps bot commented Mar 19, 2026 •

edited

Loading